Affected versions

• Firefox 67.0.2 (20190607204818)
• Firefox 68.0b9 (20190610153228)
• Firefox 69.0a1 (20190613095633)

Affected platforms

• Windows 10x64
• Windows 7 x64

Steps to reproduce

1. Install an older version of Firefox and go to About Firefox page to trigger the update download.
2. After update download is completed switch to another windows user account without closing Firefox or updating it.
3. Open Firefox on the other user and open About Firefox page.

Expected result

• “Restart to update Firefox” button is not displayed (Firefox is updated to the latest version).

Actual result

• Firefox is updated to the latest version but “Restart to update” button is displayed.

Regression Range

• I will search for one ASAP.

Additional Notes

• Attached a log with the error from browser console while “Restart to update Firefox” loop is present in About Firefox.
• Selecting “Restart to update Firefox” button on the second user and entering About Firefox window again, the button in question will still be present every time Firefox is restarted.
• The issue is reproducible using both nsIIncrementalDownload and BITS.
• Attached a screen recording with the issue after Step 2: link.

Instead of 2 Windows accounts check what happens when using the same installation, 2 profiles, and the same Windows account

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Hello Robert,

Using the same windows account and same installation and 2 different profiles leads to the same behavior presented in comment 0, Firefox is updated to the latest version but "Restart to update Firefox" button is present. Closing the first updated Firefox fixes the issue on "Restart to update Firefox" loop. Also, the same error is thrown in the browser console: "Unable to find update: TypeError: um.getUpdateAt(...) is null BrowserGlue.jsm:1941". If there is anything I can help with please let me know. Thank you!

Thanks Alexandru, could you try to find a regression range? It would be best if you used the same Windows user account to do so.

Hello Robert, sorry for the late response…

I tried finding a regression range but almost all the versions behaves the same, I mean they update to the first watershed (first watershed - Firefox45.0a1 (2015-12-09) /second watershed 56.0a1 (2017-07-31) ->versions above to latest Nightly ) and then when clicking restart to update button Firefox remains updated at first encountered watershed, depending on Firefox version.
The only different behavior encountered was with Firefox 21.0a1(2013-02-01) and some versions below where there was presented a "Software Update Failed" error when starting the second instance of Firefox and no update occurred after opening it even if Restart to Update button was selected from about Firefox menu. Attached a screenshot with the error. Should I try to find a regression range using that behavior? Or there is another way how to check for a regression range? Thank you!

The priority flag is not set for this bug.
:rstrong, could you have a look please?

For more information, please visit auto_nag documentation.

P3 since it is a longstanding bug per comment #7.

Still happening.

I'm on Windows 10 64 bit.

• had version 73.0.1 running
• opened the about dialog... it downloaded an update
• then about:profiles I launched another profile
• in the other profile the about dialog says version 74.0 and shows the "Restart to update Firefox" button. Even after quitting and reopening this second profile.

Still happening.

Firefox is updated to the latest version but “Restart to update” button is displayed.

Dialog shows version 95.0.1 (currently latest), and there is a "Restart to Update Firefox” button.
Clicking it restarts FF, but the same situation remains: the button is still there
Can click it again and the cycle just repeats.

I'm going to paste this useful comment from the bug I just duped to this one (Bug 1671003 Comment 13):

I can reproduce it now given your steps in comment 10, thank you!

I'm really not sure why I wasn't able to reproduce it before, though, these simple steps will cause it, without involving another user at all:

1. Install 81.0.1, run it from the installer
2. Open About
3. Wait until the update is downloaded and staged and the "restart to update Firefox" button appears
4. Close About
5. Create a new profile in about:profiles
6. Launch profile in new browser
7. (The update replaces the install as it starts up)
8. Open About in the new browser. This will be the new version (82.0.2), but will still say "restart to update Firefox"

Still confused about how I was trying this differently yesterday, I must have sloppily been doing something I didn't write down.

It looks like the instance that applies the update wasn't able to clean it up, so it still looks like there's an update pending, I think this is because the update mutex is still held by the instance that downloaded the update, which would cause _postUpdateProcessing to bail out before the successful updates gets cleaned up. This much makes sense, the fix for this will be bug 1553982, the update semaphore will prevent the update from occurring when any other instance would have been holding the update mutex.

I think the button is still shown is because isReadyForRestart (true because there is an applied update in the update manager, which always loads active-update.xml regardless of the mutex) is checked before isOtherInstanceHandlingUpdates in AppUpdater.check() (also in AppUpdater.status()).

:bytesized what would you think about reversing the order here, so that READY_FOR_RESTART doesn't preempt OTHER_INSTANCE_HANDLING_UPDATES?

I should note that Bug 1553982 did not end up fixing this issue because we did not opt to avoid updates when another version of Firefox is running (the main reason being that it's too easy for malware to pretend to be another instance of Firefox).

Another side effect of this problem also occurred to me. In addition to incorrectly showing the "Restart to Update" button, I don't think we would show the What's New page that we sometimes show after updates. I think that we may or may not end up eventually showing the page, depending on the exact sequence of events after we have gotten into this situation. If, for example, we shut down both browsers/profiles, and then only restart the one that was originally holding the update mutex, I think that the page will be shown. If, however, we shut both down and then start the one that was incorrectly showing the "Restart" button, then I believe the page will never be shown. I haven't actually tested these claims, I'm just reasoning about how they work.

Regarding the proposed solution:

:bytesized what would you think about reversing the order here, so that READY_FOR_RESTART doesn't preempt OTHER_INSTANCE_HANDLING_UPDATES?

I've been giving it some more thought and I'm unsure if this is still the route that I want to take. There are a few problems that I'm concerned with:

1. It doesn't address the "What's New" problem that I mention above.
2. It feels misleading to say "I'm not handling updates, that instance over there is" right after installing an update.
3. I'm uncomfortable about the situation where the profile that downloaded the update exits, leaving the second profile (with the incorrect "Restart to Update" button) to take over the update process. Off the top of my head, I'm not entirely sure what will happen in this case. It might be able to download more updates, in some circumstances, but I wouldn't count on it. Quite possibly, _postUpdateProcessing will never be called.
4. Almost certainly, profile 1 is going to eventually hit Bug 1480452. There's probably nothing that we can do about this short of properly fixing Bug 1480452, but I thought it was worth mentioning.

Also, in the course of investigating this, I was surprised that AppUpdater.isReadyForRestart returns true here at all. It essentially checks that the update state is one of the ones that indicates that an update is about to be installed. But, at this point, the update state should be "succeeded", not one of the "pending-*" or "applied-*" states that typically indicates that we are ready to restart. This seems curious and concerning. But I believe I have tracked down the underlying problem. I'm a bit embarrassed to say that I didn't entirely realize up until now that we store the update state in two different places: in update.status and in active-update.xml. Normally, we always change the state in both places at once, like this. That realization scares me quite a bit, because that situation is very fragile. We aren't even using a function that always does both things, we just make sure to always do both things every time we do it. Yikes.

The one time when these values are not kept in sync is when the updater binary runs. The updater binary doesn't touch the XML, and only worries itself with update.status. Normally, we synchronize these when _postUpdateProcessing runs. But since, in this case, _postUpdateProcessing didn't really run, the state values stay out of sync.

Perhaps, when we read active-update.xml, we should discard the state value within? Then we could map the getter for nsIUpdate.state to get the value from update.status? Or maybe a cached value of it; we read that value kind of a lot to be reading it off the disk every time. In the case of this bug, this would bypass the need to reorder the READY_FOR_RESTART and OTHER_INSTANCE_HANDLING_UPDATES cases, since the update state would now correctly show that the update is not in a "ready to install" state. This wouldn't actually solve any of the 4 problems I mentioned above, but at least we wouldn't have this state mismatch.

I'm going to try to give this some more thought and see if I can figure out a way to address at least problems (1) and (3). Those seem like the most important of those problems.

(In reply to Kirk Steuber (he/him) [:bytesized] from comment #13)

Perhaps, when we read active-update.xml, we should discard the state value within? Then we could map the getter for nsIUpdate.state to get the value from update.status? Or maybe a cached value of it; we read that value kind of a lot to be reading it off the disk every time. In the case of this bug, this would bypass the need to reorder the READY_FOR_RESTART and OTHER_INSTANCE_HANDLING_UPDATES cases, since the update state would now correctly show that the update is not in a "ready to install" state. This wouldn't actually solve any of the 4 problems I mentioned above, but at least we wouldn't have this state mismatch.

I filed Bug 1746522 for this.